docs: port ranges, OpenSearch default image, quick-start fixes, MWAA index row, parity TODOs - #2690
docs: port ranges, OpenSearch default image, quick-start fixes, MWAA index row, parity TODOs#2690sam-fakhreddine wants to merge 11 commits into
Conversation
…xy port ranges (cherry picked from commit 8424039361a5f280d4fa04aba527f22787411d17)
…DPOINT variable (cherry picked from commit efee53d0e044b8ea2200e3df6d4794a1bb89856f)
…ault (cherry picked from commit 8372880983fd7a4803d78f8e9d8389c0177d7f3e)
…iption The MWAA webserver proxy listed in the same section forwards HTTP, so describing every in-Floci proxy as a TCP proxy is inaccurate. (cherry picked from commit e362f4c5e928f1775f3530d4cd5f6c6afbe5395a)
…il nav entry MWAA has had a page and nav entry since it shipped but was never added to the services table. Separately, mkdocs.yml listed CloudTrail twice in the nav.
|
| Filename | Overview |
|---|---|
| docs/configuration/ports.md | Documents additional proxy ranges and now consistently identifies 12000–12499 as the active Lambda Runtime API range. |
| docs/getting-started/quick-start.md | Corrects the ECR endpoint variable and registry port, and removes obsolete firewall ranges. |
| docs/service-api-parity-todos.md | Adds a consolidated, evidence-oriented backlog for service API parity work. |
| docs/configuration/environment-variables.md | Correctly documents the OpenSearch image override as unset by default. |
| README.md | Aligns the OpenSearch default-image table with version-based image resolution. |
| mkdocs.yml | Removes the duplicate CloudTrail navigation entry while retaining the canonical service link. |
Reviews (6): Last reviewed commit: "docs(parity-todos): drop unreachable bra..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR batches several documentation corrections across Floci’s user/docs site: port-range references, environment variable defaults, quick-start commands, MkDocs navigation, and a new parity TODO inventory.
Changes:
- Updates port-range documentation (adds MemoryDB/Neptune/MWAA proxies; adjusts proxy wording) and quick-start examples (ECR endpoint var + registry port).
- Corrects OpenSearch default-image documentation to reflect that
FLOCI_SERVICES_OPENSEARCH_DEFAULT_IMAGEis unset by default and resolved perEngineVersion. - Adds MWAA to the services index table, removes a duplicate CloudTrail nav entry, and introduces a consolidated parity TODO document.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates documented default for FLOCI_SERVICES_OPENSEARCH_DEFAULT_IMAGE to “unset” behavior. |
| mkdocs.yml | Removes a duplicate CloudTrail navigation entry. |
| docs/services/index.md | Adds MWAA to the services operation-count table. |
| docs/service-api-parity-todos.md | New consolidated parity/TODO inventory document. |
| docs/getting-started/quick-start.md | Fixes ECR quick-start commands and adds tighter UFW examples. |
| docs/configuration/ports.md | Expands the port-range reference to cover more proxy services and adjusts proxy wording. |
| docs/configuration/environment-variables.md | Corrects OpenSearch default-image documentation to match “unset” behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…docs Address PR floci-io#2690 review findings (Greptile/Copilot): - docs/configuration/ports.md and docs/getting-started/quick-start.md still listed the retired Lambda Runtime API range (9200-9299) and a never-valid ECR range (5000-5099) alongside the current 12000-12499 and 5100-5199 ranges. Drop the stale entries. - docs/services/index.md had a duplicate MWAA row (added by 9a8a6e5) with a conflicting endpoint description; the service matrix already carried MWAA. Remove the duplicate; verified via `make docs-sync` that no other tables are stale. - docs/service-api-parity-todos.md links to four investigation epic docs (issues/0009, codebuild-local-agent, codepipeline-v2, network-data-plane) that existed on throwaway/rebase-onto-upstream but were never merged into any docs branch. Restore them from that branch so the links resolve instead of stripping the references.
pgermosen
left a comment
pgermosen
left a comment
There was a problem hiding this comment.
The six described fixes check out — I spot-checked the OpenSearch default-image claim directly against EmulatorConfig.java (the field really is an unannotated Optional, no fixed default, confirming the doc correction) and confirmed the port-range and duplicate-MWAA-row bot findings are resolved.
Before going further though, the PR summary describes six small fixes, but the diff also includes four larger files — three multi-hundred-line investigation/architecture documents and one issue-tracking doc — that aren't mentioned anywhere in the description. I see from the thread that these were restored from another branch specifically to fix broken links a new file introduced, so I understand how they got here. Three of the four (the CodeBuild local-agent, CodePipeline V2, and network-data-plane investigation docs) read as legitimate, general project documentation — decision frameworks written for the project rather than for one person — so I don't have a content objection to them, just a visibility one: a reviewer relying on the summary alone would have no idea they're part of this PR at all.
The fourth one, the account+region-scoping audit issue doc, is a different case worth a closer look. It references a workflow script, a specific automation tool, and a particular subagent/model orchestration setup, none of which exist anywhere in this repository, and its own decision log reads in the voice of one person's private working notes rather than something another contributor could pick up and act on. The underlying technical concern it describes, ambient account or region resolution being used in place of an already-known explicit value, sounds like a genuinely worthwhile thing to track for this project. But as written, nobody besides the author can act on the file, since the tooling it references doesn't exist here. Would you be open to either rewriting that one as a project-facing issue describing just the bug pattern and the affected files, or letting me know if there's a reason it should stay as-is?
…s tooling refs pgermosen (PR floci-io#2690 review) flagged that this file referenced a workflow script, a subagent/model orchestration setup, and a shell script under scripts/static-checks/ that don't exist anywhere in this repo, and read like one person's private working notes rather than something another contributor could act on. Rewrite it to describe the underlying bug pattern (ambient account/region resolution used in place of an already-resolved explicit value), cite the three confirmed fixes by commit SHA instead of nonexistent issue file paths, and replace the proprietary detection tooling with a plain shell one-liner any contributor can run. The candidate file list, service-domain scope, and the AWS global-services exclusion list are unchanged.
|
Thanks for the close look, and for spot-checking the OpenSearch claim against On visibility: you're right that the summary didn't mention the four restored files. Updated the PR description to call out that On the fourth file, Pushed as |
…s tooling refs pgermosen (PR floci-io#2690 review) flagged that this file referenced a workflow script, a subagent/model orchestration setup, and a shell script under scripts/static-checks/ that don't exist anywhere in this repo, and read like one person's private working notes rather than something another contributor could act on. Rewrite it to describe the underlying bug pattern (ambient account/region resolution used in place of an already-resolved explicit value), cite the three confirmed fixes by commit SHA instead of nonexistent issue file paths, and replace the proprietary detection tooling with a plain shell one-liner any contributor can run. The candidate file list, service-domain scope, and the AWS global-services exclusion list are unchanged.
483747e to
0784ef2
Compare
hectorvent
added
documentation
|
Thanks for the careful batch, and for the issues/0009 rewrite. The project-facing version is a real improvement, and I checked the port and image claims against the current config, they all hold up. (blocking) The problem you fixed inside issues/0009 is still present one file over. In (blocking) Could you also drop (follow-up, separate PR) Whenever suits you, would you mind a small PR retiring None of this is a comment on the content, which is genuinely useful. Happy to look again once the first two land. |
hectorvent
added
waiting-contributor
and removed
waiting-maintainer
… path PAR-001 pointed at a script that does not exist in this repository and at issues/0009, which only made sense inside a top-level issues/ tree this PR should not be adding. Inlined the candidate-search command directly and dropped the file. The completed-work rollup table cited fork-only branch names and short SHAs that are not resolvable outside this fork; dropped that column, keeping the area and required close-out.
… path PAR-001 pointed at a script that does not exist in this repository and at issues/0009, which only made sense inside a top-level issues/ tree this PR should not be adding. Inlined the candidate-search command directly. The completed-work rollup table cited fork-only branch names and short SHAs that are not resolvable outside this fork; dropped that column, keeping the area and required close-out.
|
Fixed in 94b621d — PAR-001 no longer points at the nonexistent script; the candidate-search command is inlined directly in the row. On the follow-up: happy to send a small PR retiring top-level |
|
Thank you for turning that around so quickly, and sorry for asking in two rounds rather than one. Checking at your latest commit: (blocking, small) One row survived the sweep. In (the remaining ask) Would you consider moving The six documentation fixes underneath all this are good and I would merge them today. |
|
Fixed in ab183a3 — PAR-002 no longer names a fork-only branch/commit; it now describes the implemented state directly, matching the treatment PAR-001's neighbor row already got. On moving |
Summary
Six small, independent documentation fixes, batched into one PR to avoid opening six near-trivial PRs:
docs/configuration/ports.md, which previously only covered ElastiCache and RDS.FLOCI_SERVICES_OPENSEARCH_DEFAULT_IMAGEis unset by default (images resolve per requestedEngineVersion, seeOpenSearchDomainManager.java:151/EmulatorConfig.java:1045,Optional<String> defaultImage()), butREADME.mdanddocs/configuration/environment-variables.mdboth documented a fixed default image string. Corrected both.$AWS_ENDPOINTvariable (the guide defines$AWS_ENDPOINT_URLearlier but the ECR section used the wrong name).docs/services/index.md(MWAA has had a service page and nav entry since it shipped but was never added to this specific table), and dropped a duplicateCloudTrailnav entry frommkdocs.yml.docs/service-api-parity-todos.md, consolidating scattered service API parity TODOs into one tracked document. This doc links to four investigation/epic files that existed on an unmerged branch but not on any docs branch — restoring them was needed to avoid shipping broken links: three general project docs (docs/services/codebuild-local-agent-investigation-epic.md,docs/services/codepipeline-v2-epic.md,docs/services/network-data-plane-investigation-epic.md) and one issue-tracking doc (issues/0009-epic-account-region-scoping-audit.md, rewritten to drop references to tooling that doesn't exist in this repo and read as a project-facing bug-pattern writeup instead of private notes).All content was verified as still current against
upstream/mainbefore pushing — this is a docs-only branch that had drifted since being cut, so it was rebased onto currentupstream/mainfirst, and each fix was re-checked against the current code/docs state rather than assumed still applicable.Deliberate scope notes
feature/docs, previously at a base ~128 commits behind currentmain). That earlier push contained 4 of these 6 commits (same fixes, different SHAs after rebase); the 2 newest commits (MWAA operations-table row + parity-todos consolidation) were not yet present there. No open PR existed against that branch, so it was force-updated in place rather than opened as a second competing PR..javafiles touched, so the wire-fidelity extractor and local code-review steps in the PR-maintenance runbook do not apply.Type of change
AWS Compatibility
Checklist
upstream/mainrtk mvn --ultra-compact clean test-compile— BUILD SUCCESS (no non-doc files changed)make docs-check— passes clean